feat(commands): defineCommand — typed declarative commands via an ICommand adapter - #6101
Draft
edusperoni wants to merge 19 commits into
Draft
feat(commands): defineCommand — typed declarative commands via an ICommand adapter#6101edusperoni wants to merge 19 commits into
edusperoni wants to merge 19 commits into
Conversation
Purpose-built container with an Angular-compatible surface (inject, runInInjectionContext, Injector, provide/provideLazy, forwardRef). Lookup is class-object first with a fallback to the decorator-set name, per injector level, so per-call string overrides and duplicated contract copies in the extensions tree resolve to the same provider. Includes a legacy provider kind that constructs Yok-style classes via annotate(), lazy side-effect loaders for path-based registration, transient retention, and reverse-instantiation-order disposal.
reportDeprecation() dedups per api+detail and logs at trace level by default; NS_DEPRECATIONS=warn|error previews the stricter stages so the same call sites can be escalated over releases. Wired at the external entry points only: param-name hook invocation, require-time extension registration, and dynamicCall help templating.
A class migrated off property injection has neither $hooksService nor $injector, so the decorator threw at hook-execution time. The global injector must stay last in the chain - tests stub the instance properties and rely on them winning.
Yok keeps its entire public surface, subclassability, and the global $injector, while storage and resolution delegate to the new container. Command routing, key commands, and the public-API builder are unchanged. Every legacy member now carries @deprecated JSDoc naming its replacement, mirrored on IInjector.
DoctorService and ProjectNameService become @contract abstract classes; their impls are renamed *Impl (externally invisible - outside resolution is by string name or token, never class identity). The subpath resolves through contracts/package.json rather than an exports map, so existing deep requires keep working, and the entry point is side-effect-free so a duplicated CLI copy in an extensions tree never boots a second runtime.
Fixtures exercise the surfaces third parties rely on: param-name hook signatures across every payload shape and influence channel (mutation, function-return middleware, abort), require-time extension registration against global.$injector including hierarchical commands, and the full IInjector facade surface. Notably they pin that a hook naming an unwrapped payload key (the after-watchAction shape) is skipped as invalid - resolution changes must not resurrect long-dead hooks.
dependency-injection.md covers tokens (@contract), inject()/Injector, provider kinds, resolution semantics, forwardRef, coexistence with the legacy $injector, and a legacy-to-new quick reference. extending-cli.md leads with the recommended hook pattern - inject() works directly in hook bodies because they run in an injection context, pinned by a new compat test - and demotes parameter-name injection to a labeled legacy section. String-token lookups are framed as the migration bridge, not a co-equal API. The hook deprecation tracer now flags only hooks that actually use param-name service injection; a hookArgs-only signature follows the recommended pattern and stays silent.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5 tasks
createHierarchicalCommand registered the synthesized parent - and its execute path resolved commandsService and errors - through the module-level global injector instead of the instance it was called on, so a parent dispatcher leaked onto the global injector whenever a hierarchical command was registered on any other instance. The require-ordering guard accidentally depended on that leak: with the parent now landing on the same instance, it is exempted for synthesized parents (they exist because a child registered, not because requires ran out of order), and the else branch mirrors the existing default-command skip. Net effect: register-then-require orderings that used to throw 'Default commands should be required before child commands' now work; nothing that worked before changes.
Internal code no longer reads global.$injector: the two cycle-bound sites (the deprecation tracer's logger fallback and @hook's last-resort lookup) go through a getInjector() accessor required at call time. The global property remains write-only from the CLI's side - it exists as the published surface for extensions and hooks.
edusperoni
force-pushed
the
feat/define-command
branch
from
July 30, 2026 01:39
b15734b to
863f964
Compare
The token container was reachable only through a getter on the concrete Yok class, so every crossing from facade-typed code into the new API needed an any cast - the migration's most important seam was invisible to the type system. IInjector now declares readonly di: Injector.
edusperoni
force-pushed
the
feat/define-command
branch
from
July 30, 2026 01:44
863f964 to
74caa8f
Compare
Yok is now an Injector - class Yok extends Injector - so the new API works on the facade directly (get, register with Providers, createChild, runInInjectionContext($injector, ...)) and inject(Injector) inside legacy-constructed classes returns the facade itself instead of a second, inner container identity. The di bridge is gone. register() dispatches by argument shape: a string first argument is the legacy name-based form, anything else is a Provider. IInjector now extends the Injector class type, which constrains implementers to the real class hierarchy (Yok and its subclasses) - intentional, since the interface only ever described Yok.
IInjector recomposes from per-subsystem faces - CommandRegistry, KeyCommandRegistry, ModuleRegistry, PublicApiBuilder - each an @contract token the facade registers itself under. One object still implements everything until the subsystems are physically extracted; extraction then becomes a provider swap for the face's token instead of a consumer migration. Consumers can depend on the narrow face they actually use, and deprecation becomes per-face instead of a flat everything-is-legacy. The contracts are internal (lib/common/contracts) and deliberately not re-exported from nativescript/contracts; promoting one is a per-contract decision.
- doctor-service: printWarnings accepts an optional trackResult matching its contracts; runSetupScript returns the setup script result instead of resolving undefined; canExecuteLocalBuild no longer dereferences its optional argument - deprecation tracer: a report dropped for lack of a logger is no longer latched as delivered - it reports once a logger exists - yok: global.$injector is an accessor pair, so a direct third-party assignment stays synchronized with the binding getInjector() reads - docs: note the useValue + shared:false quirk; show where Injector is imported from in the late-lookup example
edusperoni
force-pushed
the
feat/define-command
branch
from
July 30, 2026 02:27
74caa8f to
cafa737
Compare
optional resolves to null instead of throwing for unknown tokens (a found-but-misconfigured provider still throws); skipSelf starts at the parent, escaping a child scope's shadowing entry; self refuses parent fallthrough. self+skipSelf throws. host is deliberately absent - it is an Angular component-tree concept with no analog in this hierarchy. get()'s former second parameter - the legacy per-call ctorArguments bag - had exactly one caller, the facade's own resolve(name, bag). It moves to a protected getWithLegacyArguments channel, so the public get() aligns with Angular's shape today rather than after the legacy paths are deleted.
…adapter Commands can now be declared as plain objects: a name, an option schema built from booleanOption/stringOption/numberOption/arrayOption, and a run function whose context carries the positional args plus the declared options, typed by inference from the schema. lib/common/define-command holds the types and the pure factories only, so it stays side-effect-free and can be re-exported from nativescript/contracts. The runtime bridge lives in lib/common/services/command-definition-adapter, which compiles a definition into the ICommand the legacy registry expects and runs it inside an injection context. canExecute is emitted only when the definition supplies one or opts into arguments: "any"; CommandsService skips all parameter validation as soon as canExecute exists, so omitting it is what lets the framework reject stray positional arguments for arguments: "none". Fully additive — existing ICommand classes are untouched.
…nitions A definition with no declared options must be executable in a container that has no options service registered - manifest-loaded extension commands run in exactly that situation.
The parent-dispatcher leak onto the module-level injector is fixed in the base branch, so the round-trip test no longer needs the global facade.
Yok extends Injector on the base branch; the di bridge is gone.
edusperoni
force-pushed
the
feat/define-command
branch
from
July 30, 2026 02:51
cafa737 to
a1ba0ef
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
What is the current behavior?
A command is a class implementing
ICommand, registered under a stringly-typed key, reading flags off the untyped global$optionsobject. The validation semantics carry a trap: declaringcanExecutesilently disablesallowedParametersvalidation, and an emptyallowedParametersmeans "reject all positional arguments" — none of which the types express.What is the new behavior?
defineCommand— a declarative, typed command definition that plugs into the existing registry through an adapter (createCommandFromDefinition/registerCommandDefinition). Fully additive: routing, validation, help, hooks, and analytics behavior are untouched, and legacyICommandclasses remain fully supported.dashedOptions, riding the CLI's existing option validation (declared flags accepted; unknown flags hard-fail with help, exactly as today).ctx.optionsis a typed view with real inference (pinned by an exactness type test, not a strict-off-friendly assignment).canExecutetrap is handled explicitly and documented as a constraint in the adapter:arguments: "none"(default) omitscanExecuteso the framework's own no-parameters validation applies;arguments: "any"or a usercanExecutetakes validation ownership wholesale.runexecutes in an injection context, soinject()works inside commands the same as everywhere else.lib/common/define-command.tsis side-effect-free and exported fromnativescript/contracts; definitions carry aSymbol.formarker so duplicated CLI copies interoperate.CommandsService.tryExecuteCommand.defining-commands.md.Two pre-existing registry quirks surfaced while testing, unchanged by this PR and worth their own issues: the synthesized hierarchical parent registers via the module-level global injector rather than the instance it was called on, and
registerCommandalone never populateshierarchicalCommandsrouting state (onlyrequireCommanddoes).Full suite: 110 files, 1616 passed / 38 skipped; yok oracle, public-API test, and compat fixtures untouched.